Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

feat: discard patch layer #689

Merged
merged 28 commits into from
Aug 2, 2024
Merged

feat: discard patch layer #689

merged 28 commits into from
Aug 2, 2024

Conversation

MiahaCybersec
Copy link
Contributor

Closes #389

Miaha Cybersec and others added 15 commits June 27, 2024 10:07
Add mock config
e026382 
Signed-off-by: Miaha Cybersec <[email protected]>
Revert "Add mock config"
66f345d 
This reverts commit 5e3ee36.

Signed-off-by: Miaha Cybersec <[email protected]>
refactor code to unmarshal and modify image metadata
c7f8c9e 
Signed-off-by: Miaha Cybersec <[email protected]>
refactor code for readability and maintainability
b7bd8e8 
Signed-off-by: Miaha Cybersec <[email protected]>
Refactor updateImageMetadata and setupLabels functions
da366b0 
Signed-off-by: Miaha Cybersec <[email protected]>
Add support for patched image data in BuildkitConfig
20e35df 
Signed-off-by: Miaha Cybersec <[email protected]>
Add patch retention for previously patched images, code refactoring
051ffd5 
Signed-off-by: Miaha Cybersec <[email protected]>
Add patch retention for previously patched images, add tests
c6b6c31 
Signed-off-by: Miaha Cybersec <[email protected]>
Refactor tests and rename function in buildkit package
3dbb9c0 
Signed-off-by: Miaha Cybersec <[email protected]>
Expand test coverage
8f1d79b 
Signed-off-by: Miaha Cybersec <[email protected]>
@MiahaCybersec
Merge branch 'project-copacetic:main' into discard-patch-layer
53e313d
Minor refactor to discard patch layers in all package manager
27ca23e 
Signed-off-by: Miaha Cybersec <[email protected]>
Add mock client tests and expand test coverage
218cf8a 
Signed-off-by: Miaha Cybersec <[email protected]>
@MiahaCybersec
Merge branch 'project-copacetic:main' into discard-patch-layer
32889ca
@MiahaCybersec
Merge branch 'project-copacetic:main' into discard-patch-layer
ec7b989
@codecov Codecov
Copy link

codecov bot commented Jul 16, 2024 •
edited
Loading

Codecov Report

Attention: Patch coverage is 33.33333% with 66 lines in your changes missing coverage. Please review.

Project coverage is 34.22%. Comparing base (358a7ff) to head (efa23d8).

Files Patch % Lines
pkg/buildkit/buildkit.go 53.33% 23 Missing and 5 partials ⚠️
pkg/pkgmgr/dpkg.go 5.55% 15 Missing and 2 partials ⚠️
pkg/pkgmgr/rpm.go 0.00% 14 Missing ⚠️
pkg/pkgmgr/apk.go 0.00% 7 Missing ⚠️
Additional details and impacted files 
@@            Coverage Diff             @@
##             main     #689      +/-   ##
==========================================
+ Coverage   34.21%   34.22%   +0.01%     
==========================================
  Files          18       18              
  Lines        1584     1677      +93     
==========================================
+ Hits          542      574      +32     
- Misses       1011     1065      +54     
- Partials       31       38       +7

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

ashnamehrotra
ashnamehrotra requested changes Jul 18, 2024
View reviewed changes
pkg/buildkit/buildkit.go Show resolved Hide resolved
pkg/buildkit/buildkit.go Show resolved Hide resolved
pkg/buildkit/buildkit.go Outdated Show resolved Hide resolved
pkg/buildkit/buildkit.go Outdated Show resolved Hide resolved
pkg/pkgmgr/apk.go Outdated Show resolved Hide resolved
pkg/pkgmgr/dpkg.go Show resolved Hide resolved
@ashnamehrotra
Copy link
Contributor

@MiahaCybersec great work so far! I had a few changes requested, as well as bug with the filesystem when testing. Once that is addressed, lets also add docuemntation for this feature and specify that this is only for docker labels, and oci annotations are not currently supported until we can support oci exports in #604

Miaha Cybersec added 3 commits July 22, 2024 09:31
Refactor patch merging and apply error handling
d9b67ca 
Signed-off-by: Miaha Cybersec <[email protected]>
golangci-lint
28aa14e 
Signed-off-by: Miaha Cybersec <[email protected]>
golangci-lint
c27ac73 
Signed-off-by: Miaha Cybersec <[email protected]>
MiahaCybersec
MiahaCybersec commented Jul 22, 2024
View reviewed changes
pkg/pkgmgr/dpkg.go Outdated Show resolved Hide resolved
@ashnamehrotra
Copy link
Contributor

ashnamehrotra commented Jul 30, 2024 •
edited
Loading

@MiahaCybersec when testing this locally, I am seeing more layers when running "docker history" on a normal image. When running copa patch again on that, I am seeing the layers add up:

(1) docker history ashnam/nginx:1.21.6-patched

IMAGE          CREATED              CREATED BY                                      SIZE      COMMENT
9762d32fd15f   About a minute ago   mount / from exec sh -c apt install --no-ins…   53.7MB    buildkit.exporter.image.v0
<missing>      2 minutes ago        mount / from exec sh -c apt list --upgradabl…   4.1kB     buildkit.exporter.image.v0
<missing>      2 minutes ago        mount / from exec apt update                    17.9MB    buildkit.exporter.image.v0
<missing>      2 years ago          /bin/sh -c #(nop)  CMD ["nginx" "-g" "daemon…   0B        
<missing>      2 years ago          /bin/sh -c #(nop)  STOPSIGNAL SIGQUIT           0B        
<missing>      2 years ago          /bin/sh -c #(nop)  EXPOSE 80                    0B        
<missing>      2 years ago          /bin/sh -c #(nop)  ENTRYPOINT ["/docker-entr…   0B        
<missing>      2 years ago          /bin/sh -c #(nop) COPY file:09a214a3e07c919a…   16.4kB    
<missing>      2 years ago          /bin/sh -c #(nop) COPY file:0fd5fca330dcd6a7…   12.3kB    
<missing>      2 years ago          /bin/sh -c #(nop) COPY file:0b866ff3fc1ef5b0…   12.3kB    
<missing>      2 years ago          /bin/sh -c #(nop) COPY file:65504f71f5855ca0…   8.19kB    
<missing>      2 years ago          /bin/sh -c set -x     && addgroup --system -…   63.5MB    
<missing>      2 years ago          /bin/sh -c #(nop)  ENV PKG_RELEASE=1~bullseye   0B        
<missing>      2 years ago          /bin/sh -c #(nop)  ENV NJS_VERSION=0.7.3        0B        
<missing>      2 years ago          /bin/sh -c #(nop)  ENV NGINX_VERSION=1.21.6     0B        
<missing>      2 years ago          /bin/sh -c #(nop)  LABEL maintainer=NGINX Do…   0B        
<missing>      2 years ago          /bin/sh -c #(nop)  CMD ["bash"]                 0B        
<missing>      2 years ago          /bin/sh -c #(nop) ADD file:55b4fe3115c684f54…   85.8MB

whereas the current copa patch would show one layer
(2) docker history ashnam/nginx:1.21.6-patched

IMAGE          CREATED          CREATED BY                                      SIZE      COMMENT
48c66f6d60cf   14 seconds ago   mount / from exec sh -c apt install --no-ins…   53.7MB    buildkit.exporter.image.v0
<missing>      2 years ago      /bin/sh -c #(nop)  CMD ["nginx" "-g" "daemon…   0B        
<missing>      2 years ago      /bin/sh -c #(nop)  STOPSIGNAL SIGQUIT           0B        
<missing>      2 years ago      /bin/sh -c #(nop)  EXPOSE 80                    0B        
<missing>      2 years ago      /bin/sh -c #(nop)  ENTRYPOINT ["/docker-entr…   0B        
<missing>      2 years ago      /bin/sh -c #(nop) COPY file:09a214a3e07c919a…   16.4kB    
<missing>      2 years ago      /bin/sh -c #(nop) COPY file:0fd5fca330dcd6a7…   12.3kB    
<missing>      2 years ago      /bin/sh -c #(nop) COPY file:0b866ff3fc1ef5b0…   12.3kB    
<missing>      2 years ago      /bin/sh -c #(nop) COPY file:65504f71f5855ca0…   8.19kB    
<missing>      2 years ago      /bin/sh -c set -x     && addgroup --system -…   63.5MB    
<missing>      2 years ago      /bin/sh -c #(nop)  ENV PKG_RELEASE=1~bullseye   0B        
<missing>      2 years ago      /bin/sh -c #(nop)  ENV NJS_VERSION=0.7.3        0B        
<missing>      2 years ago      /bin/sh -c #(nop)  ENV NGINX_VERSION=1.21.6     0B        
<missing>      2 years ago      /bin/sh -c #(nop)  LABEL maintainer=NGINX Do…   0B        
<missing>      2 years ago      /bin/sh -c #(nop)  CMD ["bash"]                 0B        
<missing>      2 years ago      /bin/sh -c #(nop) ADD file:55b4fe3115c684f54…   85.8MB

and then re-patching the (1) without scan report:
(3) docker history ashnam/nginx:1.21.6-patched-patched

IMAGE          CREATED          CREATED BY                                      SIZE      COMMENT
d430c8f95c05   29 seconds ago   mount / from exec /bin/sh -c if [ -s error_l…   4.1kB     buildkit.exporter.image.v0
<missing>      30 seconds ago   mount / from exec sh -c output=$(apt upgrade…   12.3MB    buildkit.exporter.image.v0
<missing>      33 seconds ago   mount / from exec sh -c apt list --upgradabl…   4.1kB     buildkit.exporter.image.v0
<missing>      34 seconds ago   mount / from exec apt update                    4.1kB     buildkit.exporter.image.v0
<missing>      7 minutes ago    mount / from exec sh -c apt install --no-ins…   53.7MB    buildkit.exporter.image.v0
<missing>      8 minutes ago    mount / from exec sh -c apt list --upgradabl…   4.1kB     buildkit.exporter.image.v0
<missing>      11 months ago    mount / from exec sh -c apt install --no-ins…   17.9MB    buildkit.exporter.image.v0
<missing>      2 years ago      CMD ["nginx" "-g" "daemon off;"]                0B        buildkit.dockerfile.v0
<missing>      2 years ago      STOPSIGNAL SIGQUIT                              0B        buildkit.dockerfile.v0
<missing>      2 years ago      EXPOSE map[80/tcp:{}]                           0B        buildkit.dockerfile.v0
<missing>      2 years ago      ENTRYPOINT ["/docker-entrypoint.sh"]            0B        buildkit.dockerfile.v0
<missing>      2 years ago      COPY 30-tune-worker-processes.sh /docker-ent…   16.4kB    buildkit.dockerfile.v0
<missing>      2 years ago      COPY 20-envsubst-on-templates.sh /docker-ent…   12.3kB    buildkit.dockerfile.v0
<missing>      2 years ago      COPY 10-listen-on-ipv6-by-default.sh /docker…   12.3kB    buildkit.dockerfile.v0
<missing>      2 years ago      COPY docker-entrypoint.sh / # buildkit          8.19kB    buildkit.dockerfile.v0
<missing>      2 years ago      RUN /bin/sh -c set -x     && addgroup --syst…   63.5MB    buildkit.dockerfile.v0
<missing>      2 years ago      ENV PKG_RELEASE=1~bullseye                      0B        buildkit.dockerfile.v0
<missing>      2 years ago      ENV NJS_VERSION=0.7.0                           0B        buildkit.dockerfile.v0
<missing>      2 years ago      ENV NGINX_VERSION=1.20.2                        0B        buildkit.dockerfile.v0
<missing>      2 years ago      LABEL maintainer=NGINX Docker Maintainers <d…   0B        buildkit.dockerfile.v0
<missing>      2 years ago      /bin/sh -c #(nop)  CMD ["bash"]                 0B        
<missing>      2 years ago      /bin/sh -c #(nop) ADD file:09675d11695f65c55…   85.8MB

Revert first-time patching logic
a01880e 
Signed-off-by: Miaha Cybersec <[email protected]>
@ashnamehrotra
Copy link
Contributor

@MiahaCybersec that is resolved, thanks! I did some testing looking at the layers/labels of the images and it looks good to me.

There is one more thing - when comparing a double patched image (with scanner then update all) like nginx:1.21.6-patched-patched with discard patch layer changes vs current copa, even though current copa has more layers, the size of the resulting image is less than the size of the resulting image with discard patch layer. Are you able to reproduce this/know what could be causing that? maybe we are copying over extra files?

ashnamehrotra
ashnamehrotra reviewed Aug 2, 2024
View reviewed changes
pkg/pkgmgr/dpkg.go Outdated Show resolved Hide resolved
ashnamehrotra
ashnamehrotra approved these changes Aug 2, 2024
View reviewed changes
Copy link
Contributor

@ashnamehrotra ashnamehrotra left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thank you!

@ashnamehrotra ashnamehrotra merged commit 839ffa5 into project-copacetic:main Aug 2, 2024
24 checks passed
@MiahaCybersec MiahaCybersec deleted the discard-patch-layer branch August 2, 2024 21:21
@sozercan sozercan added this to the v0.9.0 milestone Sep 11, 2024
@ashnamehrotra ashnamehrotra modified the milestones: v0.9.0, v0.8.0 Sep 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ashnamehrotra ashnamehrotra ashnamehrotra approved these changes

@jeremyrickard jeremyrickard Awaiting requested review from jeremyrickard jeremyrickard is a code owner

@sozercan sozercan Awaiting requested review from sozercan sozercan is a code owner

Assignees
No one assigned
Labels
None yet
Projects
Copacetic Workboard
Status: ✅ Done
Milestone
v0.8.0
Development

Successfully merging this pull request may close these issues.

[REQ] discard patch layer for subsequent patches
3 participants
@MiahaCybersec @ashnamehrotra @sozercan